fix: Multibind scopes #284

eirikur-nc · 2025-10-12T23:15:49Z

What

Change how the scope argument of the multibind method works, so that it applies to the target rather than the containing list or dictionary. Also, consider explicit class bindings when resolving multibindings, and honor the scope of the class binding when the multibinding has no scope.

Why

So that it becomes possible to control the scope of individual multi-bindings. E.g.

def configure(binder: Binder) -> None:
    binder.multibind(List[Plugin], to=PluginA, scope=singleton)
    binder.multibind(List[Plugin], to=PluginB)

injector = Injector([configure])
first_list = injector.get(List[Plugin])
second_list = injector.get(List[Plugin])

assert first_list is not second_list
assert first_list[0] is second_list[0]
assert first_list[1] is not second_list[1]

Closes #283

codecov · 2025-10-12T23:16:23Z

Codecov Report

❌ Patch coverage is 98.14815% with 1 line in your changes missing coverage. Please review.
✅ Project coverage is 95.98%. Comparing base (5a30e9a) to head (9faa32a).
⚠️ Report is 14 commits behind head on master.

Files with missing lines	Patch %	Lines
injector/__init__.py	98.14%	1 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##           master     #284      +/-   ##
==========================================
+ Coverage   95.84%   95.98%   +0.13%     
==========================================
  Files           1        1              
  Lines         602      623      +21     
  Branches      103      106       +3     
==========================================
+ Hits          577      598      +21     
  Misses         19       19              
  Partials        6        6

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

injector/__init__.py

injector_test.py

davidparsson · 2025-10-24T20:53:33Z

Thank you. This looks promising. I'll try to find time to review this properly.

davidparsson · 2025-11-27T13:24:08Z

injector/__init__.py

+        # HACK: generate a pseudo-type for this element in the list.
+        # This is needed for scopes to work properly. Some, like the Singleton scope,
+        # key instances by type, so we need one that is unique to this binding.
+        pseudo_type = type(f"multibind-type-{id(provider)}", (provider.__class__,), {})


Could you please elaborate a bit on in what case this is needed? Is it required for the case when multiple different types are bound with the singleton scope in the same multibound type? Just like PluginA and PluginC in the case below:

def test_multibind_dict_scopes_applies_to_the_bound_items() -> None: def configure(binder: Binder) -> None: binder.multibind(Dict[str, Plugin], to={'a': PluginA}, scope=singleton) binder.multibind(Dict[str, Plugin], to={'b': PluginB}) binder.multibind(Dict[str, Plugin], to={'c': PluginC}, scope=singleton)

I quickly tried to reproduce the problem without the pseudo-type, by instead passing the resolved type, which is Plugin for the example above. I got a weird problem in that test case that I haven't yet resolved, but for multibound lists every case I can come up with seems to work as expected. This makes me suspect it's not really needed.

After digging in the code for I while, I rather think this is the reason why the hack is needed. Could the reason be that the scope is applied to the binding of KeyValueProvider (for example {'a': PluginA}) and not the type itself (PluginA)? Or is it rather that we want a singleton configuration (or, in other words, Injector/Binder) per multibind() call?

davidparsson

Thanks for one more nice contribution!

I'm planning on merging this, and tweaking a few minor things before releasing, but I would appreciate if you could take the time to respond to my comments.

davidparsson · 2025-11-27T14:52:51Z

injector/__init__.py

+                isinstance(binding.provider, ClassProvider)
+                and binding.scope is NoScope
+                and self._binder.parent
+                and self._binder.parent.has_explicit_binding_for(binding.provider._cls)


I'm planning on removing the has_explicit_binding_for() condition, since an explicit binding (that should be respected) may exist on parent.parent.parent as well. That change also makes sure that classes decorated with @singleton are instantiated as far up in the hierarchy as possible, which makes sure the instance is shared among all child injectors.

Do you have any argument against doing this? All tests still pass.

Sorry, but I have changed my mind. The scope applied to a binding should only be applied for that binding. This is how it works for regular bindings, so this is how it should work for multibinds as well. The special case checking for explicit bindings (above) should be removed.

However, @singleton decorators on classes should ideally be respected, since that is respected for regular binds. However, the pseudo type currently prevents this, and the KeyValueProvider also makes this tricky. I have been trying to get this done today, but still without success.

I have removed the logic above and merged it already, and also added a failing test case for the @singleton case in #292.

I would have really liked to get a release out today, but I don't think I can do it, and it might very well take a while before I have a larger chunk of time to spend on this again.

davidparsson · 2025-11-27T14:55:38Z

injector/__init__.py

+        # HACK: generate a pseudo-type for this element in the list.
+        # This is needed for scopes to work properly. Some, like the Singleton scope,
+        # key instances by type, so we need one that is unique to this binding.
+        pseudo_type = type(f"multibind-type-{id(provider)}", (provider.__class__,), {})


I quickly tried to reproduce the problem without the pseudo-type, by instead passing the resolved type, which is Plugin for the example above. I got a weird problem in that test case that I haven't yet resolved, but for multibound lists every case I can come up with seems to work as expected. This makes me suspect it's not really needed.

injector/__init__.py

eirikur-nc · 2025-11-28T09:24:21Z

Thanks for one more nice contribution!

I'm planning on merging this, and tweaking a few minor things before releasing, but I would appreciate if you could take the time to respond to my comments.

I'll put aside some time on Sunday for this.

davidparsson · 2025-11-28T10:10:40Z

I'll put aside some time on Sunday for this.

Thank you! Hopefully will only take a short while.

However, I'm confident enough to go ahead and merge this, and since I have some time today I might release this as well.

eirikur-nc added 7 commits October 12, 2025 09:22

add tests, format test file

11042c7

Slightly tweak one test

bdbf66c

Get scopes working for list binder

1032e80

Defer to parent binder if it has an explicit binding

efc62e1

Make test a bit harder

4d57f11

add scope support to dicts

b4d9fcb

rename test

37e071e

eirikur-nc commented Oct 12, 2025

View reviewed changes

injector/__init__.py Show resolved Hide resolved

eirikur-nc commented Oct 12, 2025

View reviewed changes

injector/__init__.py Show resolved Hide resolved

eirikur-nc commented Oct 12, 2025

View reviewed changes

injector/__init__.py Show resolved Hide resolved

eirikur-nc commented Oct 12, 2025

View reviewed changes

injector_test.py Show resolved Hide resolved

eirikur-nc added 3 commits October 13, 2025 22:35

Add more information to a comment

11b52e2

Extract method

2d45015

format

9faa32a

eirikur-nc marked this pull request as ready for review October 13, 2025 23:18

davidparsson mentioned this pull request Nov 20, 2025

Release new version #288

Open

davidparsson reviewed Nov 27, 2025

View reviewed changes

davidparsson approved these changes Nov 28, 2025

View reviewed changes

davidparsson merged commit 0256913 into python-injector:master Nov 28, 2025
13 checks passed

fix: Multibind scopes #284

fix: Multibind scopes #284

Uh oh!

Conversation

eirikur-nc commented Oct 12, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

What

Why

Uh oh!

codecov bot commented Oct 12, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

davidparsson commented Oct 24, 2025

Uh oh!

davidparsson Nov 27, 2025

Choose a reason for hiding this comment

Uh oh!

davidparsson Nov 27, 2025

Choose a reason for hiding this comment

Uh oh!

davidparsson Nov 28, 2025

Choose a reason for hiding this comment

Uh oh!

davidparsson left a comment

Choose a reason for hiding this comment

Uh oh!

davidparsson Nov 27, 2025

Choose a reason for hiding this comment

Uh oh!

davidparsson Nov 28, 2025

Choose a reason for hiding this comment

Uh oh!

davidparsson Nov 28, 2025

Choose a reason for hiding this comment

Uh oh!

davidparsson Nov 27, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

eirikur-nc commented Nov 28, 2025

Uh oh!

davidparsson commented Nov 28, 2025

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

eirikur-nc commented Oct 12, 2025 •

edited

Loading

codecov bot commented Oct 12, 2025 •

edited

Loading